-
Notifications
You must be signed in to change notification settings - Fork 423
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
UCP/API: Add recv_info to ucp_request_param_t #5919
Conversation
src/ucp/api/ucp.h
Outdated
* in case of immediate completion of receive operation. | ||
*/ | ||
union { | ||
size_t *length; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need uint32_t mask
here to show which fields were set by UCP?
e.g. I'd not expect that tag
is updated by UCP AM API, but a user could specify it (e.g. user shares the same request params for TAG and AM operations)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i do not think so. This is somewhat similar to cb
field of this struct
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, this is union
I see
@yosefe, @shamisp, @tonycurtis, can you please take a look? |
src/ucp/api/ucp.h
Outdated
* in case of immediate completion of receive operation. | ||
*/ | ||
union { | ||
size_t *length; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please add description for each one of the fields. I guess user has to allocate the memory and then it is updated in function return ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added
src/ucp/api/ucp.h
Outdated
*/ | ||
union { | ||
size_t *length; | ||
ucp_tag_recv_info_t *tag; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
variable name tag
is very confusing. Why not info
, seems like this is the var name used everywhere else.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to avoid double info
, because it is a part of recv_info
union, so it will be recv_info.tag_info
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI, we use info
everywhere else, so I have no issue with double naming. tag_info
is double-double naming :) If you have strong preference towards tag_info
I can accept it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
👍 |
bot:pipe:retest |
@tonycurtis, can you please review? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1423: in the case of an immediate...
1423: The user has to ...
1432: previous comment split into 2 sentences, should be consistent here
2827: value to which it points ... of the received...
3396: ditto
fixed |
@tonycurtis, do you have other comments? |
a3fd63c
to
d8b829e
Compare
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
bot:pipe:retest |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
What
Add
recv_info
toucp_request_param_t
, so that*recv_nbx
function can get receive data details in case of immediate completion.Why ?
If operation completes immediately the callback is not invoked. Need to provide the length (and other info) of received data to the user.